feat: Added ui feedback when adding type in typesync#512
feat: Added ui feedback when adding type in typesync#512unniznd wants to merge 5 commits intographprotocol:mainfrom
Conversation
cmwhited
left a comment
cmwhited
left a comment
There was a problem hiding this comment.
Appreciate you taking this task on!
Few things:
- make sure to format and lint your code
- I am not sure the typeRefs map and useEffect is the best way to scroll the newly added type into view. could just add the scroll into view in the
KnowledgeGraphBrowser.typeSelectedinstead. much less state to manage and theuseEffecthas me really worried
|
@cmwhited thank you for your feedback. I will work on it. |
|
@cmwhited |
what errors is it giving you? |
|
I have fixed the issue in linting. I was having a package installation issue. I have updated the code. Can you review and give some feedback @cmwhited |
| // Create a ref to store DOM references for each type | ||
| const typeRefs = useRef<Map<number, HTMLDivElement>>(new Map()); | ||
| // State to track the index of the newly added type | ||
| const [_newTypeIndex, setNewTypeIndex] = useState<number | null>(null); |
There was a problem hiding this comment.
If we aren't reading the state variable here: _newTypeIndex, which I am assuming we are not because you prepended it with an underscore, then why do we need to set the state at all? It isn't being used, is it?
| }); | ||
|
|
||
| // Create a ref to store DOM references for each type | ||
| const typeRefs = useRef<Map<number, HTMLDivElement>>(new Map()); |
There was a problem hiding this comment.
This still does not feel like the right answer to me. Especially not the way it is being set
|
@cmwhited, I have updated the code. Can you review and give some feedback? |
2 participants
UI Feedback when you added a new type from the left panel
Closes #498